fix(env): check all shell profiles in vp env doctor IDE integration#878
fix(env): check all shell profiles in vp env doctor IDE integration#878MahmoodKhalil57 wants to merge 1 commit intovoidzero-dev:mainfrom
vp env doctor IDE integration#878Conversation
The install script writes env sourcing to .zshrc, .bashrc, .bash_profile, and .profile, but the doctor only checked .zshenv and .profile (macOS) or .profile alone (Linux). This caused false negatives where the doctor reported IDE integration wasn't set up even when it was. Check all profiles the install script may have written to, matching the same set used by `vp implode` for cleanup. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
✅ Deploy Preview for viteplus-preview canceled.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 84eb7fb40f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| #[cfg(not(any(target_os = "macos", target_os = "linux")))] | ||
| let profile_files: &[&str] = &[".profile"]; | ||
| // Check all profiles the install script may have written to | ||
| let profile_files: &[&str] = &[".zshenv", ".zshrc", ".bash_profile", ".bashrc", ".profile"]; |
There was a problem hiding this comment.
check_profile_files now treats .zshrc and .bashrc as sufficient for passing "IDE integration", but those files are for interactive shells rather than GUI app environments (packages/cli/install.sh:350-372), and the doctor guidance itself still tells Linux users to configure ~/.profile for IDEs (print_ide_setup_guidance, doctor.rs:516-520). In setups where only .bashrc is present/updated, vp env doctor will report IDE integration as configured and suppress the IDE setup warning even though desktop-launched IDEs can still miss the PATH change.
Useful? React with 👍 / 👎.
fengmk2
left a comment
fengmk2
left a comment
There was a problem hiding this comment.
Thanks for the fix! The mismatch between install.sh and doctor.rs is a real problem, but I think this particular change introduces false positives that are worse than the false negatives it fixes.
The issue
The "IDE integration" check (line 507) is specifically about whether GUI-launched IDEs (e.g. VS Code opened from Finder/dock/app launcher) can see the vite-plus PATH. GUI apps don't run through an interactive shell, so they only pick up environment from specific files:
-
macOS: Only
.zshenv(sourced for all zsh invocations) and.profile(read by login shells / display managers) reliably affect GUI apps..zshrcand.bashrcare only sourced for interactive shells — an IDE launched from the dock won't read them. -
Linux: The display manager (GDM, SDDM, etc.) typically sources
.profileor.xprofile. Files like.bashrcand.zshrcare interactive-shell-only and won't be seen by a GUI-launched IDE.
What this PR does
By checking all 5 profile files, the doctor will now report ✓ IDE integration: env sourced in ~/.bashrc with a green checkmark — even though the IDE won't actually read .bashrc. That's a false positive: the user thinks IDE integration is set up, but it isn't.
Suggestion
The original platform-specific lists were correct for the IDE check's purpose. The real problem is that install.sh writes to files that help with terminal usage (.bashrc, .zshrc) but not necessarily IDE integration. A couple of options:
- Keep the strict IDE-relevant file list for the IDE check, but add a separate "shell integration" check that covers the broader set of files.
- Improve
install.shto also write to IDE-relevant files (e.g. ensure.zshenvor.profileis always written to, not just.zshrc/.bashrc).
## Summary - Introduce tri-state `EnvSourcingStatus` (`IdeFound` / `ShellOnly` / `NotFound`) to replace the boolean `check_ide_integration()` return - Check IDE-visible profiles (`.zshenv`, `.profile`) first, then fall back to all shell profiles (`.bashrc`, `.zshrc`, `.bash_profile`, fish configs) - Add proper fish shell support with `env.fish` pattern matching via `is_fish` flag - Parameterize `check_profile_files()` to accept profile list as argument for reuse This avoids the false positives that caused #878 to be rejected (marking `.bashrc` as IDE-OK) while eliminating the false negatives from #881 (ignoring shell-only sourcing entirely). Closes #881 --------- Co-authored-by: MK (fengmk2) <fengmk2@gmail.com>
2 participants
Summary
.zshrc,.bashrc,.bash_profile, and.profilevp env doctoronly checked.zshenv+.profile(macOS) or.profilealone (Linux)Fixes #881
Now checks all profiles the install script may have written to, matching the same set used by
vp implodefor cleanup.Context
install.shwrites to:.zshenv+.zshrc.bash_profile+.bashrc+.profiledoctor.rsonly checked:.zshenv,.profile.profileSo a Linux user running zsh (env in
.zshrc) or bash (env in.bashrc) got a false negative.